fix: return reverted field of logs set to true if tipset was reverted#7172
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds reorg-removed log emission for Changeseth_subscribe reorg-removed logs pipeline
Sequence Diagram(s)sequenceDiagram
participant ChainHeadChanges
participant run_logs_feed
participant eth_logs_for_head_change
participant collect_events_from_messages
participant LogsFeed
participant spawn_logs
participant log_matches
participant SubscriptionSink
ChainHeadChanges->>run_logs_feed: head change event
run_logs_feed->>eth_logs_for_head_change: PathChange<Tipset>
eth_logs_for_head_change->>collect_events_from_messages: collect with revert status
collect_events_from_messages-->>eth_logs_for_head_change: CollectedEvent list
eth_logs_for_head_change-->>run_logs_feed: Vec<EthLog>
run_logs_feed->>LogsFeed: broadcast batch
LogsFeed-->>spawn_logs: shared batch
spawn_logs->>log_matches: filter by EthFilterSpec
log_matches-->>spawn_logs: matched logs
spawn_logs->>SubscriptionSink: forward logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
akaladarshi
force-pushed
the
akaladarshi/return-logs-reverted
branch
from
acbad44 to
7141458
Compare
akaladarshi
requested review from
LesnyRumcajs and
hanabi1224
and removed request for
a team
akaladarshi
force-pushed
the
akaladarshi/return-logs-reverted
branch
2 times, most recently
from
1d0b496 to
e1043a0
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/rpc/methods/eth.rs`:
- Around line 3318-3320: The `eth_getFilterLogs` method is incorrectly using the
`poll_event_filter` helper which mutates internal state by updating
`seen_positions` at lines 3353-3361. This makes the method non-idempotent,
causing repeated calls to return different results and breaking subsequent
`eth_getFilterChanges` calls. Instead of calling `poll_event_filter`, directly
collect the filter's full result set from the canonical chain without updating
any poll state. Keep the `poll_event_filter` usage only in
`eth_getFilterChanges` where state mutation is appropriate. Extract the core
logic for collecting events from the filter without the state mutation part.
In `@src/state_manager/state_computation.rs`:
- Around line 70-84: The load_executed_tipset_with_receipt method is bypassing
the stale-cache validation and removal guard that is implemented in the
load_executed_tipset_with_cache method. This causes stale cached ExecutedTipset
entries to be returned after head reset or garbage collection events. Apply the
same cache validity checking and invalidation logic that exists in
load_executed_tipset_with_cache (Lines 99-107) to the
load_executed_tipset_with_receipt method before or after the get_or_insert_async
call to ensure stale entries are properly removed from the cache in the
receipt-specific flow.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 9018da5a-761b-49ea-8e98-3507519807fa
📒 Files selected for processing (14)
CHANGELOG.mdsrc/daemon/mod.rssrc/rpc/methods/chain.rssrc/rpc/methods/eth.rssrc/rpc/methods/eth/filter/event.rssrc/rpc/methods/eth/filter/mod.rssrc/rpc/methods/eth/pubsub.rssrc/rpc/methods/sync.rssrc/rpc/mod.rssrc/state_manager/state_computation.rssrc/tool/offline_server/server.rssrc/tool/subcommands/api_cmd/generate_test_snapshot.rssrc/tool/subcommands/api_cmd/stateful_tests.rssrc/tool/subcommands/api_cmd/test_snapshot.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
filecoin-project/lotus(manual)
3 tasks
akaladarshi
force-pushed
the
akaladarshi/return-logs-reverted
branch
from
a967d70 to
33c3fb8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files
... and 11 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
akaladarshi
force-pushed
the
akaladarshi/return-logs-reverted
branch
from
33c3fb8 to
9a4b513
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/state_manager/state_computation.rs (1)
63-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAlign receipt-path cache reads with stale-cache invalidation logic
load_executed_tipset_with_receiptbypasses the stale-cache validation used byload_executed_tipset_with_cache, so it can return invalid cached entries after head reset/GC. Please apply the same pre-read validation/removal guard here beforeget_or_insert_async(Line 63-Line 77).Suggested patch
pub async fn load_executed_tipset_with_receipt( &self, msg_ts: &Tipset, receipt_ts: &Tipset, ) -> anyhow::Result<ExecutedTipset> { + // Keep cache-validity behavior consistent with load_executed_tipset_with_cache. + if msg_ts.epoch() >= self.heaviest_tipset().epoch() + && let Some(cached) = self.cache.get(msg_ts.key()) + { + if StateTree::new_from_root(self.db(), &cached.state_root).is_ok() { + return Ok(cached); + } else { + self.cache.remove(msg_ts.key()); + } + } + self.cache .get_or_insert_async(msg_ts.key(), async move { self.load_executed_tipset_inner( msg_ts, Some(receipt_ts), Self::rpc_state_recompute_policy(), ) .await }) .await }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/state_manager/state_computation.rs` around lines 63 - 77, The load_executed_tipset_with_receipt method does not perform the same stale-cache validation that load_executed_tipset_with_cache applies before reading from cache, which allows invalid cached entries to persist after head reset or garbage collection operations. Apply the same pre-read validation and removal guard logic to load_executed_tipset_with_receipt before the get_or_insert_async call to ensure stale cache entries are properly invalidated before retrieval.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@src/state_manager/state_computation.rs`:
- Around line 63-77: The load_executed_tipset_with_receipt method does not
perform the same stale-cache validation that load_executed_tipset_with_cache
applies before reading from cache, which allows invalid cached entries to
persist after head reset or garbage collection operations. Apply the same
pre-read validation and removal guard logic to load_executed_tipset_with_receipt
before the get_or_insert_async call to ensure stale cache entries are properly
invalidated before retrieval.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4aa9f30f-5851-46ce-8802-e0a7032902b1
📒 Files selected for processing (14)
CHANGELOG.mdsrc/daemon/mod.rssrc/rpc/methods/chain.rssrc/rpc/methods/eth.rssrc/rpc/methods/eth/filter/event.rssrc/rpc/methods/eth/filter/mod.rssrc/rpc/methods/eth/pubsub.rssrc/rpc/methods/sync.rssrc/rpc/mod.rssrc/state_manager/state_computation.rssrc/tool/offline_server/server.rssrc/tool/subcommands/api_cmd/generate_test_snapshot.rssrc/tool/subcommands/api_cmd/stateful_tests.rssrc/tool/subcommands/api_cmd/test_snapshot.rs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
filecoin-project/lotus(manual)
✅ Files skipped from review due to trivial changes (4)
- src/tool/offline_server/server.rs
- src/tool/subcommands/api_cmd/generate_test_snapshot.rs
- CHANGELOG.md
- src/rpc/methods/sync.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- src/tool/subcommands/api_cmd/test_snapshot.rs
- src/daemon/mod.rs
- src/rpc/methods/chain.rs
- src/rpc/methods/eth/filter/event.rs
- src/rpc/mod.rs
- src/rpc/methods/eth.rs
- src/tool/subcommands/api_cmd/stateful_tests.rs
- src/rpc/methods/eth/pubsub.rs
- src/rpc/methods/eth/filter/mod.rs
akaladarshi
force-pushed
the
akaladarshi/return-logs-reverted
branch
from
627bda7 to
d788dc5
Compare
LesnyRumcajs
approved these changes
Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes #7096
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit
eth_subscribelog delivery during reorgs so reorg-reverted logs are re-emitted withremoved: true, before logs from the replacing chain head.eth_getFilterLogs/eth_getFilterChanges) to return only newly observed events by tracking per-filter event delivery positions.eth_subscribecoverage into a filter-matrix validating address/topic wildcard behavior (including trailing wildcards) and correct inclusion/exclusion across concurrent subscriptions.